asset marketplace #84
Conversation
jburnt
force-pushed
the
feat/marketplace
branch
from
f44c450 to
4090295
Compare
Peartes
force-pushed
the
feat/xion-asset
branch
from
be618c1 to
d0c79a7
Compare
Signed-off-by: Kehinde Faleye <[email protected]>
add approve/reject
|
This marketplace contract needs to be enhanced to support Crossmint secondary sales. Crossmint requires the ability to purchase NFTs on behalf of end users, where Crossmint pays for the transaction but the NFT is delivered directly to the customer's wallet.
The existing BuyItem {
listing_id: String,
price: Coin,
},The current implementation in A new |
| @@ -0,0 +1,2535 @@ | |||
| use anyhow::Error; | |||
There was a problem hiding this comment.
This needs to be broken down into more specific files. It is currently unwieldy
| pub listing_denom: String, | ||
| } | ||
|
|
||
| // Maximum fee bps allowed. |
There was a problem hiding this comment.
"bps" needs to be defined somewhere
| } | ||
|
|
||
| #[cfg_attr(not(feature = "library"), cosmwasm_std::entry_point)] | ||
| pub fn execute( |
There was a problem hiding this comment.
A nit, but if we pulled query into it's own file, why not execute?
| // remove listing | ||
| listings().remove(deps.storage, listing_id.clone())?; | ||
|
|
||
| let buy_msg = asset_buy_msg(info.sender.clone(), listing.token_id.clone()); |
There was a problem hiding this comment.
As @emperorjm mentioned, this requires the buyer to be the msg sender. This will not work for things like crossmint, gift purchasing, etc. Like the buy handler for the asset, we should accept an optional receiver, and if it is unset, use the info.sender
There was a problem hiding this comment.
Also, this logic seems to have no enforcement of marketplace fees. How do the marketplace and asset work together for fees?
| listing.token_id.clone(), | ||
| price, | ||
| None, | ||
| None, |
There was a problem hiding this comment.
Should we add fee info here?
| pub fee_recipient: T, | ||
| pub sale_approvals: bool, | ||
| pub fee_bps: u64, | ||
| pub listing_denom: String, |
There was a problem hiding this comment.
This means that marketplaces can only list in one denom, which is probably fine for the first version. In the future, we will need to figure out how to reason about multi-denom marketplaces
No description provided.